-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add experimental support for Thread Sanitizer #2076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@rjmccall, @gribozavr please, take a look! |
@AnnaZaks Library changes LGTM! |
@@ -297,7 +297,7 @@ template <class EntryTy> class ConcurrentMap { | |||
|
|||
// Try to set the edge to the new node. | |||
if (std::atomic_compare_exchange_strong_explicit(edge, &node, newNode, | |||
std::memory_order_release, | |||
std::memory_order_acq_rel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but I'm going to link to C++ DR 2445 in protest. :)
Otherwise, runtime/IRGen changes LGTM. |
Can we separate the "enable tsan" changes from the "fix stuff detected by tsan" changes? |
|
||
// Currently, more than one sanitizer cannot be enabled at the same time. | ||
if (pKind != SanitizerKind::None && pKind != kind) { | ||
SmallVector<char, 128> pb; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: SmallString?
@gparker42 They are all separate commits, which you can view individually. |
@swift-ci Please test |
This patch threads the TSan option through the front end.
This bug was reported by TSan.
Make sure the memory synchronization ordering on success is strictly stronger than the memory ordering of failure. This addresses a race reported by TSan when having both Swift tests and the runtime TSanified.
This data race is benign and should not occur on the platforms we currently care about. However, do set the ordering appropriately when built with TSan to avoid reporting races from Swift code. The metadata lookup code relies on these orderings for synchronization.
It is safe to only reset the counter when the count is non-zero. We always initialize the count to zero. This race has been reported by TSan.
@swift-ci Please test |
Awesome! |
Thread the -sanitize=thread option through the front end and fix a bunch of races it reported.